Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use IndexedDB to store Thunderstore package list #1261

Merged
merged 5 commits into from
Apr 15, 2024
Merged

Conversation

anttimaki
Copy link
Collaborator

@anttimaki anttimaki commented Mar 12, 2024

Draft PR for requesting early feedback. Cleaned up the PR, and good to go from my perspective. Note that #1266 was prepended to this PR stack.

Refreshing the contents of the IndexedDB on the SplashScreen takes about 12 secs for Lethal Company. I'd say ~66% of that is throwing out the old values, so we might consider some alternative way of dealing with mod removals? Otherwise the changes didn't seem to affect performance much one way or another (except when the IndexedDB is refreshed again by the background update). EDIT: actually there's notable but not catastrophic slowdown when rendering local mod list for the first time, currently investigating this.

Copy link
Collaborator

@MythicManiac MythicManiac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't review the refactoring code, just the dexie store so far

yarn.lock Outdated Show resolved Hide resolved
src/r2mm/manager/PackageDexieStore.ts Outdated Show resolved Hide resolved
src/r2mm/manager/PackageDexieStore.ts Outdated Show resolved Hide resolved
v3.2 brought in some new goodies, although I'm not sure if we can fully
utilise them due to old Vue version.
This IndexedDB wrapper is used for storing the responses received when
querying Thunderstore API for package listings for a specific game.

The refactoring shouldn't be visible to users (except for a minor delay
when storing the response) for now. In short term this allows us to
drop the duplicate in-memory list of ThunderstoreMod objects. In medium
term this aims to make it easier to do further refactorings to deal
with the poor performance of handling the very large mod lists in
communities like Lethal Company.
The process of accessing the list of mods available via Thunderstore
API for a specific game:

1. Components (SplashMixin on the foreground and UtilityMixin on the
   background) trigger the API request, currently via
   ConnectionProvider
2. Components pass the response to ThunderstorePackages which writes
   the results to IndexedDB. ThunderstorePackages module might become
   obsolete in the future
3. Components trigger Vuex update where TsModsModule reads the mod list
   from IndexedDB and stores it in memory
4. Component access the list via Vuex

To keep the commit size down ModBridge and ThunderstorePackages'
internal methods still use the duplicate PACKAGES and PACKAGES_MAP
objects, but those will be cleaned out in the subsequent commits.
The methods isn't really used much and this allows us to drop the
dependency on the soon obsolete ModBridge.
Implement cached "ManifestV2 to ThunderstoreMod" conversion in
TsModsModule in a way that's compatible with what ModBridgre did
previously. This allows us to drop ModBridge, and the duplicate
in-memory list of PACKAGES.
@anttimaki anttimaki marked this pull request as ready for review April 2, 2024 11:39
Copy link
Collaborator

@MythicManiac MythicManiac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't spot anything obviously wrong with this, good to thoroughly test of course.

I don't see this PR impacting it but locally importing a mod that does not exist in the Thunderstore API is probably a good idea to check still works after these refactorings.

const cacheKey = `${mod.getName()}-${mod.getVersionNumber()}`;

if (state.cache.get(cacheKey) === undefined) {
const tsMod = state.mods.find((m) => m.getFullName() === mod.getName());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not be able to query this from the IndexedDB now? Maybe you added that in a later PR already & I forgot

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accessing IndexedDB is async and the sync Vuex getter is called by e.g. sync Vue computed properties. But I can look into it (not blocking this PR). At the very least the prewarming of cache could be done that way, although that would still require keeping the whole list in memory for other accessing code paths.

Base automatically changed from ts-mods-module to develop April 15, 2024 05:54
@anttimaki anttimaki merged commit 2882b5b into develop Apr 15, 2024
7 checks passed
@anttimaki anttimaki deleted the indexed-package-db branch April 15, 2024 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants